Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Merged by Bors] - Gltf animations #3751

Closed
wants to merge 9 commits into from
Closed

[Merged by Bors] - Gltf animations #3751

wants to merge 9 commits into from

Conversation

mockersf
Copy link
Member

@mockersf mockersf commented Jan 23, 2022 •
edited
Loading

Objective

  • Load informations for animations from GLTF
  • Make experimenting on animations easier

Non Objective

  • Implement a solutions for all animations in Bevy. This would need a discussion / RFC. The goal here is only to have the information available to try different APIs

Solution

  • Load animations with a representation close to the GLTF spec
  • Add an example to display animations. There is an animation driver in the example, not in Bevy code, to show how it can be used. The example is cycling between examples from the official gltf sample (AnimatedTriangle, BoxAnimated), and one from me with some cases not present in the official examples.
animated.mp4

@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Jan 23, 2022
@alice-i-cecile alice-i-cecile added A-Animation Make things move and change over time C-Feature A new feature, making something new possible and removed S-Needs-Triage This issue needs to be labelled labels Jan 23, 2022
StarArawn
StarArawn reviewed Jan 24, 2022
View reviewed changes
examples/3d/load_gltf_animation.rs Outdated

// This animation driver is not made to work in the general case. It will work with only one
// animation per scene, and will ignore the specified interpolation method to only do linear.
fn gltf_animation_driver(
Copy link
Contributor

@StarArawn StarArawn Jan 24, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably worth pointing out here that this isn't completely implemented per the spec. It in fact only animates transforms for GLTF nodes and will not animated bones or morphs which are also apart of the spec. Bones are treated as nodes in GLTF as well. I don't see any changes to the GLTF loader to load the skin information. I think this is a great PR though and I don't think those things should hold back it! We should just make it clear what's not currently supported here. 🤔

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skin will be added in #2359, and morphs in #3722. This PR should work directly for skin, but morph will require additional work to be animated, currently they will log a warning when loading a file with morph animation

@james7132 james7132 mentioned this pull request Feb 23, 2022
Open
14 tasks
superdump
superdump requested changes Mar 3, 2022
View reviewed changes
Copy link
Contributor

@superdump superdump left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bit of review. Not so thorough (I didn't look at the animation spec for glTF.)

examples/3d/load_gltf_animation.rs Outdated Show resolved Hide resolved
examples/3d/load_gltf_animation.rs Outdated Show resolved Hide resolved
crates/bevy_gltf/src/loader.rs Outdated Show resolved Hide resolved
crates/bevy_gltf/src/loader.rs Outdated Show resolved Hide resolved
examples/3d/load_gltf_animation.rs Outdated Show resolved Hide resolved
crates/bevy_gltf/src/lib.rs Outdated Show resolved Hide resolved
crates/bevy_gltf/src/lib.rs Outdated Show resolved Hide resolved
crates/bevy_gltf/src/loader.rs Outdated Show resolved Hide resolved
james7132
james7132 approved these changes Mar 7, 2022
View reviewed changes
Copy link
Member

@james7132 james7132 left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. This should allow easy conversion into variable-rate animation curves rather easily. Main things I'm a bit concerned with is the memory use of uncompressed values and the complexity of this initial example, but that can be addressed in a later PR.

@kirusfg kirusfg mentioned this pull request Mar 14, 2022
Open
cart
cart approved these changes Mar 22, 2022
View reviewed changes
Copy link
Member

@cart cart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. I think its important to make it clear that the example is a "manual" implementation of an animation player and not a final bevy animation api, so I renamed it and added a doc comment explaining the situation.

@cart
Copy link
Member

cart commented Mar 22, 2022

bors r+

bors bot pushed a commit that referenced this pull request Mar 22, 2022
@mockersf @cart
Gltf animations (#3751)
fbe7a49 
# Objective

- Load informations for animations from GLTF
- Make experimenting on animations easier

# Non Objective

- Implement a solutions for all animations in Bevy. This would need a discussion / RFC. The goal here is only to have the information available to try different APIs

## Solution

- Load animations with a representation close to the GLTF spec
- Add an example to display animations. There is an animation driver in the example, not in Bevy code, to show how it can be used. The example is cycling between examples from the official gltf sample ([AnimatedTriangle](https://github.com/KhronosGroup/glTF-Sample-Models/tree/master/2.0/AnimatedTriangle), [BoxAnimated](https://github.com/KhronosGroup/glTF-Sample-Models/tree/master/2.0/BoxAnimated)), and one from me with some cases not present in the official examples.


https://user-images.githubusercontent.com/8672791/150696656-073403f0-d921-43b6-beaf-099c7aee16ed.mp4




Co-authored-by: Carter Anderson <mcanders1@gmail.com>
@bors
Copy link
Contributor

bors bot commented Mar 22, 2022

Pull request successfully merged into main.

Build succeeded:

@bors bors bot changed the title Gltf animations [Merged by Bors] - Gltf animations Mar 22, 2022
@bors bors bot closed this Mar 22, 2022
aevyrie pushed a commit to aevyrie/bevy that referenced this pull request Jun 7, 2022
@mockersf @cart @aevyrie
Gltf animations (bevyengine#3751)
008bc87 
# Objective

- Load informations for animations from GLTF
- Make experimenting on animations easier

# Non Objective

- Implement a solutions for all animations in Bevy. This would need a discussion / RFC. The goal here is only to have the information available to try different APIs

## Solution

- Load animations with a representation close to the GLTF spec
- Add an example to display animations. There is an animation driver in the example, not in Bevy code, to show how it can be used. The example is cycling between examples from the official gltf sample ([AnimatedTriangle](https://github.com/KhronosGroup/glTF-Sample-Models/tree/master/2.0/AnimatedTriangle), [BoxAnimated](https://github.com/KhronosGroup/glTF-Sample-Models/tree/master/2.0/BoxAnimated)), and one from me with some cases not present in the official examples.


https://user-images.githubusercontent.com/8672791/150696656-073403f0-d921-43b6-beaf-099c7aee16ed.mp4




Co-authored-by: Carter Anderson <mcanders1@gmail.com>
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
@mockersf @cart @ItsDoot
Gltf animations (bevyengine#3751)
0540eb3 
# Objective

- Load informations for animations from GLTF
- Make experimenting on animations easier

# Non Objective

- Implement a solutions for all animations in Bevy. This would need a discussion / RFC. The goal here is only to have the information available to try different APIs

## Solution

- Load animations with a representation close to the GLTF spec
- Add an example to display animations. There is an animation driver in the example, not in Bevy code, to show how it can be used. The example is cycling between examples from the official gltf sample ([AnimatedTriangle](https://github.com/KhronosGroup/glTF-Sample-Models/tree/master/2.0/AnimatedTriangle), [BoxAnimated](https://github.com/KhronosGroup/glTF-Sample-Models/tree/master/2.0/BoxAnimated)), and one from me with some cases not present in the official examples.


https://user-images.githubusercontent.com/8672791/150696656-073403f0-d921-43b6-beaf-099c7aee16ed.mp4




Co-authored-by: Carter Anderson <mcanders1@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@StarArawn StarArawn StarArawn left review comments

@superdump superdump superdump requested changes

@james7132 james7132 james7132 approved these changes

@cart cart cart approved these changes

Assignees
No one assigned
Labels
A-Animation Make things move and change over time C-Feature A new feature, making something new possible
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@mockersf @cart @superdump @james7132 @StarArawn @alice-i-cecile